fix(verify): use scoped targetDir for Skills component paths by decode2 · Pull Request #902 · Gentleman-Programming/gentle-ai

decode2 · 2026-06-16T03:51:52Z

Summary

Fixes post-apply verification for \ComponentSkills\ when --scope=workspace\ is used. The verifier was checking skill paths in the global home directory instead of the workspace-scoped directory, causing 33 false failures.

Root Cause

In \componentPathsWithWorkspaceScoped\ (\internal/cli/run.go), the \ComponentSkills\ case passed \homeDir\ instead of the already-computed \ argetDir\ to \skills.SkillPathForAgent. Every other component case correctly uses \ argetDir.

The install path at line 788 already uses \ argetDir\ correctly:
\\go
targetDir := componentInjectionDirScoped(s.homeDir, s.workspaceDir, s.scope, adapter)
skills.Inject(targetDir, adapter, skillIDs)
\\

But the verify path used \homeDir:
\\go
path := skills.SkillPathForAgent(homeDir, adapter, skillID) // BUG
\\

Fix

One-line change: \homeDir\ → \ argetDir\ in the \ComponentSkills\ case.

Tests

Added \TestComponentPathsWorkspaceScopedSkillsUsesWorkspaceDir\ — verifies that with \ScopeWorkspace, skill paths are under the workspace dir and NOT under the home dir.
Updated \TestComponentPathsOpenClawSkillsSkipsSDDPhaseSkills\ — the test was passing an unused \ .TempDir()\ as workspaceDir but expecting home paths. OpenClaw always uses workspaceDir when set (per \componentInjectionDirScoped), so the test now reflects the correct behavior.

Closes #785

Summary by CodeRabbit

Bug Fixes

Fixed skill path resolution so workspace-scoped per-skill verification and backup target paths are derived from the provided workspace directory (rather than the home-scoped location).
Updated and added tests to verify OpenClaw skills are included under the workspace scope while SDD phase skills are excluded, and to ensure no equivalent home-scoped skill paths are selected when only workspace-scoped skills exist.
Added an integration test covering --scope workspace, confirming installed skill files are written under the workspace .claude/skills/ path only.

coderabbitai · 2026-06-16T03:52:04Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ef1fbfaa-089e-46ba-8524-216e7ce0c48d

📥 Commits

Reviewing files that changed from the base of the PR and between df96239 and f41b956.

📒 Files selected for processing (1)

internal/cli/run_integration_test.go

📝 Walkthrough

Walkthrough

In componentPathsWithWorkspaceScoped, the model.ComponentSkills branch now calls skills.SkillPathForAgent with targetDir instead of homeDir, aligning skill path resolution with the workspace/global injection scope. Unit tests are updated to verify this scoped path behavior at the component-path level, and a new integration test validates the complete install workflow respects the workspace scope when writing skill files.

Changes

ComponentSkills scope fix and tests

Layer / File(s)	Summary
Fix skill path resolution to use `targetDir` `internal/cli/run.go`	The `ComponentSkills` loop in `componentPathsWithWorkspaceScoped` now derives each skill's path from `targetDir` instead of `homeDir`, making resolved paths respect the active workspace or global scope.
Unit and integration tests for workspace-scoped skill paths `internal/cli/run_component_paths_test.go`, `internal/cli/run_integration_test.go`	`TestComponentPathsOpenClawSkillsSkipsSDDPhaseSkills` is updated to assert paths exist under a workspace temp dir. New `TestComponentPathsWorkspaceScopedSkillsUsesWorkspaceDir` asserts that `ScopeWorkspace` returns skill paths under the workspace directory and excludes home-scoped equivalents. `TestInstallWorkspaceScopeVerificationWithNoGlobalSkills` is extended with regression checks confirming workspace-scope verification includes workspace skill paths and excludes corresponding home paths. New integration test `TestRunInstallWorkspaceScopeVerification` exercises `RunInstall` with `--scope workspace`, asserting that skill files are written under the workspace directory and not under the home directory.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(verify): use scoped targetDir for Skills component paths' directly and specifically describes the one-line fix applied to the code.
Linked Issues check	✅ Passed	The PR fully addresses issue `#785`: fixing post-apply verification for ComponentSkills when using --scope=workspace by replacing homeDir with targetDir in componentPathsWithWorkspaceScoped, accompanied by comprehensive test coverage.
Out of Scope Changes check	✅ Passed	All changes (one-line fix, test corrections, new tests, and integration test) are directly aligned with fixing workspace-scoped verification for ComponentSkills in issue `#785`.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

The ComponentSkills case in componentPathsWithWorkspaceScoped passed homeDir instead of the already-computed targetDir to skills.SkillPathForAgent. When --scope=workspace, the installer writes skills to the workspace directory but post-apply verification checked the global home directory, causing false failures. The install path at run.go:788 already uses targetDir correctly, so this aligns verify with install behavior. Also fixes TestComponentPathsOpenClawSkillsSkipsSDDPhaseSkills which passed an unused workspace dir but expected home paths — OpenClaw always uses workspaceDir when set, so the test now reflects that. Closes Gentleman-Programming#785

Alan-TheGentleman

I’m requesting changes because the regression coverage stops at the internal path helper and does not assert the user-visible install --scope=workspace verification behavior from #785. Please add a focused test that runs the install path with workspace scope, no global skill files, and proves post-apply verification stays ready while checking workspace skill paths. Also run gofmt on the touched Go file before updating the PR.

@Alan-TheGentleman

…ntleman-Programming#785 Verifies that post-apply verification checks workspace paths (not home paths) when --scope=workspace is used and no global skill files exist. Also applies gofmt to internal/cli/run.go. Addresses review feedback from @Alan-TheGentleman

decode2 · 2026-06-19T22:09:34Z

Changes requested have been applied:

✅ Added regression test for workspace-scoped verification with no global skills
✅ Ran gofmt on the modified file

The test verifies that when using --scope=workspace, the verification checks workspace paths and excludes home paths, preventing false failures.

Ready for re-review.

decode2 · 2026-06-19T22:35:25Z

@Alan-TheGentleman Changes requested have been applied and are ready for re-review.

…ation

decode2 · 2026-06-23T04:29:04Z

Hi @Alan-TheGentleman, I've added the requested test:

Created a high-level integration test (\TestRunInstallWorkspaceScopeVerification\ in \internal/cli/run_integration_test.go) that executes the actual \install --scope=workspace\ workflow.
Verified that skill files are written exclusively to the workspace directory, ensuring post-apply verification succeeds when no global skills are present.

Ready for re-review!

decode2 force-pushed the fix/verify-workspace-scope-skills branch from b0e43b3 to 2e130cc Compare June 16, 2026 05:51

Alan-TheGentleman added the type:bug Bug fix label Jun 17, 2026

Alan-TheGentleman requested changes Jun 17, 2026

View reviewed changes

decode2 requested a review from Alan-TheGentleman June 17, 2026 21:55

test(cli): add integration test for install --scope=workspace verific…

f41b956

…ation

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(verify): use scoped targetDir for Skills component paths#902

fix(verify): use scoped targetDir for Skills component paths#902
decode2 wants to merge 3 commits into
Gentleman-Programming:mainfrom
decode2:fix/verify-workspace-scope-skills

decode2 commented Jun 16, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 16, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

Alan-TheGentleman left a comment

Uh oh!

decode2 commented Jun 19, 2026

Uh oh!

decode2 commented Jun 19, 2026

Uh oh!

decode2 commented Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

decode2 commented Jun 16, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root Cause

Fix

Tests

Summary by CodeRabbit

Bug Fixes

Uh oh!

coderabbitai Bot commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

Alan-TheGentleman left a comment

Choose a reason for hiding this comment

Uh oh!

decode2 commented Jun 19, 2026

Uh oh!

decode2 commented Jun 19, 2026

Uh oh!

decode2 commented Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

decode2 commented Jun 16, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 16, 2026 •

edited

Loading